-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Add plugin dir to developer jetty:run classpath for plugin dev #8033
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## main #8033 +/- ##
=============================================
- Coverage 28.57% 13.92% -14.66%
+ Complexity 29784 10820 -18964
=============================================
Files 5100 3105 -1995
Lines 358565 303890 -54675
Branches 52316 52337 +21
=============================================
- Hits 102464 42305 -60159
- Misses 241968 256123 +14155
+ Partials 14133 5462 -8671
Flags with carried forward coverage won't be shown. Click here to find out more. see 3272 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
@@ -154,7 +154,7 @@ | |||
<cs.jaxws.version>2.3.2-1</cs.jaxws.version> | |||
<cs.jersey-client.version>2.26</cs.jersey-client.version> | |||
<cs.jetty.version>9.4.51.v20230217</cs.jetty.version> | |||
<cs.jetty-maven-plugin.version>9.4.27.v20200227</cs.jetty-maven-plugin.version> | |||
<cs.jetty-maven-plugin.version>9.4.51.v20230217</cs.jetty-maven-plugin.version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 9.4.27 was a known/working version, I'm not sure if with more recent working the jetty-maven-plugin works @mlsorensen have you tested it locally (see most Github actions jobs are failing)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It worked locally, and it's the version of jetty that is bundled with the production cloudstack, but perhaps it is incompatible with the testing/PR process in some way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like the new versions of jetty may be more aggressive at scanning? I see more duplicate classes found, seems to be searching current directory as well as finding them in client/target/cloud-client-ui
jar. Will try a few things...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok I think maybe I found the issue @rohityadavcloud. Newer jetty-maven-plugin versions add the pom dependencies to classpath automatically, with older jetty we have defined extraClasspath
and add a built cloud-client-ui.jar location. This results in a lot of duplicate classes discovered. Also notice that due to using the pom dependencies for building classpath, without specifying -Psimulator
the jetty:run doesn't have simulator available which breaks the simulator CI build.
We can add explicit simulator dependency for the jetty-maven-plugin, or we can add -Psimulator
to the CI (but I don't know how to do the latter).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Eh, I'm not confident that I know exactly what's going on here, but I think the same PR in jetty that I need for this feature is probably the same one that cloudstack's use of jetty:run is incompatible with. Or certainly the very next release after this.
May need to resolve the underlying reason why we can't upgrade to newer jetty-maven-plugin before we consider adding a plugin dir for devs.
@mlsorensen smoketests (github actions) are still failing |
@mlsorensen I think this is incompatible version and may require some additional changes to make them work. Does it work for you locally? |
Description
This PR is a simple change to allow adding plugins to CloudStack when running in developer mode. Simply drop plugin jar into
client/target/plugins
before launchingmvn -pl :cloud-client-ui jetty:run
.Also bumping the version of jetty-maven-plugin to the same as main jetty server.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity